Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove webConsolePlugin from ToolchainClusterConfig #1068

Merged
merged 5 commits into from
Aug 2, 2024

Conversation

xcoulon
Copy link
Contributor

@xcoulon xcoulon commented Jul 26, 2024

Pendo has been decommisioned, we don't need this plugin anymore

Signed-off-by: Xavier Coulon <[email protected]>
xcoulon added a commit to xcoulon/toolchain-e2e that referenced this pull request Jul 26, 2024
Pendo has been decommisioned, we don't need this plugin anymore

testing for host-operator: codeready-toolchain/host-operator#1068
same changes as in codeready-toolchain#1018

Signed-off-by: Xavier Coulon <[email protected]>
Copy link
Contributor

@alexeykazakov alexeykazakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall but please do not update the go.mod dependency in this PR.

go.mod Outdated
Comment on lines 34 to 36
replace github.com/codeready-toolchain/api => github.com/xcoulon/api v0.0.0-20240725145329-0fc7541fe19e

replace github.com/codeready-toolchain/toolchain-common => github.com/xcoulon/toolchain-common v0.0.0-20240725145759-056fcf98e945
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xcoulon as you know we don't update CRDs and dependencies to api/common in the same PR. The reason is that you can't merge api and CRD changes in this same time if you do it.
You would have to:

  1. merge API
  2. change your operator/CRD PR to update the dependency (remove replace)
  3. wait for the operator/CRD PR to get green
  4. merge it

steps 2 and 3 can take time (especially if there are problems with e2e tests, like infra outage) and you risk blocking others.
It's much safer to update CRDs only, so you can merge the API and the operator PR in the same time and then create a new PR to update the dependency. So you don't block anyone who might need to make a different change to API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, got it! (I totally forgot about these constraints)

here are the PR which only deal with the removal in the CRD:
#1069
codeready-toolchain/member-operator#589
codeready-toolchain/toolchain-e2e#1021

xcoulon added a commit to xcoulon/toolchain-e2e that referenced this pull request Aug 1, 2024
Pendo has been decommisioned, we don't need this plugin anymore

testing for host-operator: codeready-toolchain/host-operator#1068
same changes as in codeready-toolchain#1018

Signed-off-by: Xavier Coulon <[email protected]>
@xcoulon
Copy link
Contributor Author

xcoulon commented Aug 1, 2024

/test e2e

Signed-off-by: Xavier Coulon <[email protected]>
Copy link

sonarcloud bot commented Aug 1, 2024

Copy link

openshift-ci bot commented Aug 1, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexeykazakov, ranakan19, xcoulon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [alexeykazakov,ranakan19,xcoulon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@alexeykazakov
Copy link
Contributor

/lgtm

Copy link

codecov bot commented Aug 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.06%. Comparing base (372ff4b) to head (1a77424).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1068      +/-   ##
==========================================
- Coverage   85.08%   85.06%   -0.02%     
==========================================
  Files          55       55              
  Lines        5034     5034              
==========================================
- Hits         4283     4282       -1     
- Misses        580      581       +1     
  Partials      171      171              

see 1 file with indirect coverage changes

@xcoulon xcoulon merged commit 9fee36c into codeready-toolchain:master Aug 2, 2024
11 of 13 checks passed
@xcoulon xcoulon deleted the remove_pendo_host branch August 2, 2024 08:08
xcoulon added a commit to codeready-toolchain/toolchain-e2e that referenced this pull request Aug 2, 2024
Pendo has been decommisioned, we don't need this plugin anymore

testing for host-operator: codeready-toolchain/host-operator#1068
same changes as in #1018

---------

Signed-off-by: Xavier Coulon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants